From: Jonathan Lebon Date: Fri, 7 Mar 2025 21:49:27 +0000 (-0500) Subject: boot: Drop ostree-finalize-staged.path X-Git-Tag: archive/raspbian/2025.7-2+rpi1^2^2~6^2~5^2~7^2 X-Git-Url: https://dgit.raspbian.org/%22http:/www.example.com/cgi/%22https:/www.github.com/%22bookmarks:///%22http:/www.example.com/cgi/%22https:/www.github.com/%22bookmarks:/?a=commitdiff_plain;h=2b9912e9f9210a3fa8396485a91224620b2138bc;p=ostree.git boot: Drop ostree-finalize-staged.path This effectively reverts ac1a919f ("boot: Add ostree-finalize-staged.path"). A bug came in on the OCP side that demonstrates that the way things are setup right now is racy. If a reboot is triggered quickly after staging a deployment, the whole pipeline of: - ostree-finalize-staged.path, which triggers - ostree-finalize-staged.service, which triggers - ostree-finalize-staged-hold.service, may not fully have happened before systemd isolates to `reboot.target` which will want to kill all pending jobs. Just directly starting the systemd unit is less elegant but much more explicit and gets rid of any possible race because it's directly part of the staging operation. Fixes: https://issues.redhat.com/browse/OCPBUGS-51150 --- diff --git a/Makefile-boot.am b/Makefile-boot.am index c07b6b81..7b7cb892 100644 --- a/Makefile-boot.am +++ b/Makefile-boot.am @@ -40,7 +40,6 @@ systemdsystemunit_DATA = src/boot/ostree-prepare-root.service \ src/boot/ostree-remount.service \ src/boot/ostree-boot-complete.service \ src/boot/ostree-finalize-staged.service \ - src/boot/ostree-finalize-staged.path \ src/boot/ostree-finalize-staged-hold.service \ src/boot/ostree-state-overlay@.service \ $(NULL) @@ -69,7 +68,6 @@ EXTRA_DIST += src/boot/dracut/module-setup.sh \ src/boot/mkinitcpio \ src/boot/ostree-boot-complete.service \ src/boot/ostree-prepare-root.service \ - src/boot/ostree-finalize-staged.path \ src/boot/ostree-remount.service \ src/boot/ostree-finalize-staged.service \ src/boot/ostree-finalize-staged-hold.service \ diff --git a/src/boot/ostree-finalize-staged.path b/src/boot/ostree-finalize-staged.path deleted file mode 100644 index 43ef0cae..00000000 --- a/src/boot/ostree-finalize-staged.path +++ /dev/null @@ -1,26 +0,0 @@ -# Copyright (C) 2018 Red Hat, Inc. -# -# This library is free software; you can redistribute it and/or -# modify it under the terms of the GNU Lesser General Public -# License as published by the Free Software Foundation; either -# version 2 of the License, or (at your option) any later version. -# -# This library is distributed in the hope that it will be useful, -# but WITHOUT ANY WARRANTY; without even the implied warranty of -# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU -# Lesser General Public License for more details. -# -# You should have received a copy of the GNU Lesser General Public -# License along with this library. If not, see . - -# For some implementation discussion, see: -# https://lists.freedesktop.org/archives/systemd-devel/2018-March/040557.html -[Unit] -Description=OSTree Monitor Staged Deployment -Documentation=man:ostree(1) - -[Path] -PathExists=/run/ostree/staged-deployment - -[Install] -WantedBy=multi-user.target diff --git a/src/libostree/ostree-impl-system-generator.c b/src/libostree/ostree-impl-system-generator.c index 6968c738..4bf3fb52 100644 --- a/src/libostree/ostree-impl-system-generator.c +++ b/src/libostree/ostree-impl-system-generator.c @@ -111,10 +111,6 @@ require_internal_units (const char *normal_dir, const char *early_dir, const cha if (!glnx_shutil_mkdir_p_at (normal_dir_dfd, "multi-user.target.wants", 0755, cancellable, error)) return FALSE; - if (symlinkat (SYSTEM_DATA_UNIT_PATH "/ostree-finalize-staged.path", normal_dir_dfd, - "multi-user.target.wants/ostree-finalize-staged.path") - < 0) - return glnx_throw_errno_prefix (error, "symlinkat"); if (symlinkat (SYSTEM_DATA_UNIT_PATH "/ostree-boot-complete.service", normal_dir_dfd, "multi-user.target.wants/ostree-boot-complete.service") < 0) diff --git a/src/libostree/ostree-sysroot-deploy.c b/src/libostree/ostree-sysroot-deploy.c index 2d8705d5..f722e754 100644 --- a/src/libostree/ostree-sysroot-deploy.c +++ b/src/libostree/ostree-sysroot-deploy.c @@ -3788,6 +3788,15 @@ ostree_sysroot_stage_tree_with_options (OstreeSysroot *self, const char *osname, if (booted_deployment == NULL) return glnx_prefix_error (error, "Cannot stage deployment"); + const char *const systemctl_argv[] + = { "systemctl", "start", "ostree-finalize-staged.service", NULL }; + int estatus; + if (!g_spawn_sync (NULL, (char **)systemctl_argv, NULL, G_SPAWN_SEARCH_PATH, NULL, NULL, NULL, + NULL, &estatus, error)) + return FALSE; + if (!g_spawn_check_exit_status (estatus, error)) + return FALSE; + g_autoptr (OstreeDeployment) deployment = NULL; if (!sysroot_initialize_deployment (self, osname, revision, origin, opts, &deployment, cancellable, error)) diff --git a/tests/kolainst/destructive/staged-deploy.sh b/tests/kolainst/destructive/staged-deploy.sh index 19881489..a268a5cc 100755 --- a/tests/kolainst/destructive/staged-deploy.sh +++ b/tests/kolainst/destructive/staged-deploy.sh @@ -12,7 +12,6 @@ case "${AUTOPKGTEST_REBOOT_MARK:-}" in sed -i -e 's,gpg-verify=true,gpg-verify=false,' /etc/ostree/remotes.d/*.conf # Test our generator - test -f /run/systemd/generator/multi-user.target.wants/ostree-finalize-staged.path test -f /run/systemd/generator/local-fs.target.requires/ostree-remount.service cat >/etc/systemd/system/sock-to-ignore.socket << 'EOF' @@ -49,11 +48,8 @@ EOF ostree commit --no-bindings --parent="${commit}" -b staged-deploy -I --consume t newcommit=$(ostree rev-parse staged-deploy) orig_mtime=$(stat -c '%.Y' /sysroot/ostree/deploy) - systemctl show -p SubState ostree-finalize-staged.path | grep -q waiting systemctl show -p ActiveState ostree-finalize-staged.service | grep -q inactive - systemctl show -p TriggeredBy ostree-finalize-staged.service | grep -q path ostree admin deploy --stage staged-deploy - systemctl show -p SubState ostree-finalize-staged.path | grep running systemctl show -p ActiveState ostree-finalize-staged.service | grep active new_mtime=$(stat -c '%.Y' /sysroot/ostree/deploy) test "${orig_mtime}" != "${new_mtime}"